Optimize leading ranges on composite sort keys - #29513
Merged
XuPeng-SH merged 3 commits intoSep 30, 2026
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
4 tasks done
XuPeng-SH
added a commit
that referenced
this pull request
Sep 30, 2026
…ety (#29518) ## What type of PR is this? - [x] BUG - [x] Improvement - [x] Documentation - [x] Test and CI ## Which issue(s) this PR fixes: Related to #29507: first-column range on a composite sort key misses object-level pruning. Backport of #29513 (merged commit `76862df5c89e8868bed5963c428f65b9eafb34b3`) to `4.2-dev`, validated against base `1a44f3db3056e32975fde86ca0ab69a094996a3f`. This updates the existing backport; it does not close an additional issue. ## What this PR does / why we need it: Composite leading-range pruning preserves the SQL row predicate while pruning compound keys. The backport also keeps metadata scan slots separate from physical column identity, resolves partition predicates against the actual partition column, and honors `CannotFold()` so volatile bounds cannot independently evaluate in row and prefix filters. The repair completes DECIMAL256 support instead of disabling its optimization: - Use the existing 33-byte DECIMAL256 tuple encoding in `serial`, `serial_full`, `SerialHelper`, extraction, and encoded-length contracts. Keep decimal scales compatible while allowing ordinary integer literals to reach leading-range pruning. - Resolve `serial_full` pack functions lazily after a NULL argument becomes non-NULL, including reuse/reset. Reject negative dynamic extraction indices in both reverse consumers. - Preserve the exact scan qualifier in the existing `ColRef.TblName` field through remapping, copying and protobuf round trips, so aliases and physical column names containing dots resolve correctly without changing EXPLAIN formatting. Metadata and row-level primary-key consumers share the same physical-name resolver; obsolete first-dot parsing and scan/relation position assumptions are removed. - Complete legacy unique-key serialization, nullable single-key compaction and primary-key bitmap alignment for DECIMAL256. Exercise the public FK LOAD fallback through Parquet, with forced index/reference comparisons and DML. ### Upgrade requirement **Existing indexes written by a legacy DECIMAL256 writer require rebuilding from the base table in a maintenance window before user reads/writes resume.** This is not an online rolling-upgrade protocol. Ordinary indexed LOAD often rejected DECIMAL256, but FK LOAD can reach a legacy writer that omitted components or rows; the reader cannot reconstruct those missing values from old keys. The delivered [upgrade procedure](https://github.com/XuPeng-SH/matrixone/blob/ee54c414323535dbed33d941ac0d397f723ebdce/docs/cn/decimal256_index_upgrade.md) documents backup, stopping all old CNs and writers, per-account inventory, preserving original index definitions, rebuilding, verification and rollback. [The inventory SQL](https://github.com/XuPeng-SH/matrixone/blob/ee54c414323535dbed33d941ac0d397f723ebdce/etc/decimal256_index_inventory.sql) conservatively lists regular indexes on DECIMAL256 tables, including indexes that use a DECIMAL256 PK suffix. The inventory also includes RTREE/SPATIAL; it does not auto-execute destructive DDL. The upgrade was challenged with the exact original PR production sources: FK Parquet LOAD succeeded under the old binary; the repaired reader on the same data directory returned no row through the old index while the base scan returned the qualifying row. Existing DROP/CREATE rebuilt the index from the base table, restored matching results, and passed UPDATE/DELETE/NULL checks. Typed single-column UNIQUE is also checked as an unaffected control. ### Validation All commands used Go 1.26.4, `GOWORK=off`, readonly module resolution, the worktree's own native build and an explicit build temporary directory. - Own native build: `make -j8 cgo`; final service: `make build` — PASS. - Full ordinary owning/direct-consumer tests via `.agents/skills/mo-dev/scripts/mo-cgo-test -p=2 -v -count=1 -timeout=600s`: `pkg/sql/plan/function`, `pkg/sql/plan`, `pkg/partitionprune`, `pkg/vm/engine/readutil`, `pkg/sql/util`, `pkg/sql/colexec/hashbuild`, `pkg/sql/colexec/runtimefilter` — PASS. - After adding the index compaction repair: full `pkg/sql/util` with `-vet=all`, plus full `pkg/sql/colexec/preinsertunique` and `pkg/sql/colexec/preinsertsecondaryindex` ordinary tests — PASS. The three new index tests were observed failing before their production repair and passing afterward. - Full vet for the seven original packages — PASS. Incremental configured golangci-lint over those packages plus both preinsert consumers, `--new-from-rev 1a44f3d` — PASS, 0 issues. The initial lint attempt timed out; it is not counted as a pass. The bounded retry used lower concurrency and completed. The three index packages were also rechecked after their production sources stabilized: PASS, 0 issues. - Serializer tests cover signed physical extrema, cross-word values, ordering, round trips, NULL contracts, reset/reuse and invalid extraction indices. Planner/readutil tests cover actual folding, integer scale metadata, dotted aliases, protobuf identity and truncated compound zonemaps without false negatives. - Persisted optimized/reference comparison: 96 comparisons across bigint/DECIMAL64/128/256, `<`, `<=`, `>`, `>=`, three bounds and ordinary/dotted aliases — PASS. This ran before the final index-only compaction additions; those additions have separate UT/BVT coverage. - Production binary for commit 7cf (SHA256 `51cbf5d71b64edc8919f65698aa170bc23bf07fe41622840771c1b226e15292f`), fresh one-CN local service, fixed golden **two normal BVT rounds on the same instance**, with no issue-skipping flag: `optimizer/composite_range_decimal.test` 78/78, `optimizer/blockfilter.test` 55/55, `join/leftjoin.sql` 68/68; **201/201 each round**, no ignored or abnormal statements. All non-built-in database/table catalog counts were zero between rounds and afterward. The new 78-statement golden and two Parquet fixtures were independently checked against SQL contracts before accepting the comparison results. - Reusing an older test directory failed WAL replay (`IOEntry[3004,2]`); the exact original PR binary failed identically on that directory. Both failures were retained as failures. The final two BVT rounds used a fresh directory and completed. - Independent QA and final incremental review for the earlier repair: **gpt-6.1-sol, xhigh**, approved tree `4b171afd8fbc437121f5547695b1711faf544a74` / delivered commit `e0c6bb5871c658c1fc394665814141df9e45e994`. All identified production blockers were repaired and re-reviewed. No new concurrency owner, worker, wait or unbounded cache was introduced; no new full race-suite pass is claimed. The prior backport's full planner race timeout on both candidate and clean base remains historical non-pass evidence. - Subsequent deep review, performed by the main agent without subagents as requested, delivered commit `7cf0409fa04888725223b2dc1335d377fd3977ac` / reviewed tree `0b87c487a75e88d95c177a7ef411b43440b420bf`: unify both row-PK consumers with the existing qualifier resolver; reduce production code by 14 lines; remove dead eligibility test logic; require fold errors and cleanup; cover invalid extraction in both decimal and string decoders; make the maintenance sequence executable by saving inventory/DDL before stopping old CNs. - After the test cleanup, full ordinary planner/function/readutil packages passed again (4.231s/13.390s/1.947s), configured three-package incremental lint passed with 0 issues. After the final row-PK source edit, full readutil tests with `-vet=all` passed again (1.942s), and configured readutil incremental lint passed with 0 issues. Six dotted-alias subcases failed before the PK repair; all 18 identity/input-shape subcases passed afterward, including non-PK controls and different scan/relation positions. - Commit 7cf production binary, independent public SQL oracle: **59 assertions passed**. This repeats the exact DECIMAL(40,0) comment on empty/non-empty tables, checks persisted fractional DECIMAL256 comparisons with real dotted column/alias names, both invalid VARCHAR extraction consumers, atomic rollback after duplicate-key batches in both orders, nullable unique rows, and real simple-PK equality/IN/range/non-PK controls. The two BVT rounds above were also repeated after this last production edit. - Comments were re-read: the single technical issue comment is addressed by complete DECIMAL256 support and its exact SQL reproduction. The other comment says automated review is paused; it is not an approval. ### Adversarial black-box / white-box QA follow-up Delivered commit `ee54c414323535dbed33d941ac0d397f723ebdce`, reviewed tree `56911cd3a2ae06fbea088e37f18caaeeac95caf4`; main-agent QA, without subagents as requested. Historical independent approval above applies to its named tree, not to this new tree. **Found and repaired a wrong-result path:** a six-row raw DECIMAL256 primary-key table with an integer UNIQUE index returns a row through FORCE INDEX before flush, but drops it after flush while the base scan returns it. Both ordinary INSERT and FK Parquet LOAD reproduced it, including small values; the exact original PR production binary also reproduced INSERT + flush. This is a pre-existing gap that full DECIMAL256 support must close. Raw DECIMAL256 intentionally has no initialized min/max zonemap because its values do not fit the existing layout. Runtime IN during index back-lookup incorrectly treated those absent statistics as no match. A 21-line guard in the existing readutil compiler conservatively retains raw DECIMAL256 blocks/objects and leaves exact row filtering in charge. AND can still prune using other supported columns; OR cannot discard the unknown branch. Serialized VARCHAR compound-key pruning remains enabled. No new format, state machine, storage path, or concurrency owner is introduced. Final service binary SHA256 `8e58b686cda62a198bd2587e71d250bca3664c010f0ff7ec11af5109651f426f`: - **534** optimized/reference comparisons across nine type families and query shapes, including signed/unsigned boundaries, DECIMAL64/128/256, float, string, datetime/timestamp, reversed operands, range/OR/NOT, joins, aggregates, ORDER/LIMIT, derived/CTE, dotted aliases. **144** independent Python numeric oracles and **8** prepared-plan parameter reuses passed; session timezone reuse was also compared. - **40** public RANGE/LIST partition comparisons against an unpartitioned reference passed, deliberately colliding compact scan positions with physical partition-column positions. - **8195 rows per writer**, modern INSERT vs FK Parquet LOAD, compound unique/secondary indexes, raw DECIMAL256 PK with nullable single-column UNIQUE: **91 assertions** passed across flush, bitmap/batch boundaries, forced index/reference results, transaction update/rollback, duplicate rejection/atomicity, prepared NULL/value INSERT reuse, malformed typed extraction, and post-error health. - Independent math/big oracle sorts **163** signed DECIMAL256 values, checks tuple byte ordering, and challenges real truncated compound zonemaps in **33** objects: **3168** no-false-negative range-pruning checks passed on the final production code. The exploratory property test stays outside the PR to avoid duplicating a test framework. - New raw-DEC256 owning regression: 8 operators × primary/non-primary columns, AND/OR composition and NULL fallback controls. The 16 operator subcases failed before the guard and passed afterward. Full readutil with vet passed after final test cleanup (1.673s); runtimefilter/hashbuild passed on the same production change; configured incremental readutil lint passed with **0 issues**. One unconfigured CGo lint attempt failed and is not counted as a pass. - Fixed golden BVT, same fresh local service, **two normal rounds**: composite_range_decimal **81/81**, blockfilter **55/55**, leftjoin **68/68**; **204/204 each round**, no failed/ignored/abnormal statements. Non-built-in database/table counts were zero after each round. The added persistence check has manually derived expected rows, not regenerated golden. The large-test draft initially passed BLOB input from UNHEX to serial_extract, failing type checking before decoding. The final test casts to the accepted VARCHAR input and verifies actual truncated/invalid markers, constant/row extraction and encoded NULL; only that final result is counted. Historical WAL replay failures and baseline failures remain failures. No new throughput benchmark or full race pass is claimed. Iceberg CI is explicitly excluded by the requester. On previous commit `7cf0409fa04888725223b2dc1335d377fd3977ac`, run `36674804116` passed preflight, shared build, SCA, UT coverage, Coverage, Mongo, PROXY and PESSIMISTIC. Ubuntu UT failed only at `pkg/tests/features.TestTableFeatures` shared three-CN cluster initialization (`waitAnyShardReadyLocked/context deadline exceeded`), before table-feature assertions; this is retained as a failure rather than declared unrelated without a base comparison. A targeted local `TestTableFeatures` run with an isolated TMPDIR passed on the new production tree; this does not prove a clean-base comparison or turn the failed remote job green. The new SHA must receive its own CI results. No new full-repository CI success is claimed. The corresponding main-branch volatile-bound fix remains a separate follow-up.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Which issue(s) this PR fixes:
Fixes #29507
What this PR does / why we need it:
A range on the first component of a composite primary or cluster key did not produce a hidden sort-key predicate for object pruning. The scan could load metadata for objects whose sort-key zone map already ruled them out.
The planner adds a supplemental hidden-key block filter for compatible bounded ranges,
BETWEEN, and one-sided comparisons, including reversed operands. The original SQL row predicate remains authoritative. The change uses the existing composite-key serializer,BlockFilterList, and readutil's sort-key zone-map fast path. First-component paired bounds stay separate, avoiding a premerge cast that could change bound semantics.Block filters now retain metadata-only column references without adding those columns to the scan reader. Row-needed columns keep their compact positions; omitted metadata columns receive distinct positions for the combined runtime/block zonemap path. This also removes unnecessary row reads for existing composite-part block filters. The relation's full schema still supplies physical sequence numbers and zonemaps; no new reader, storage format, or persistent state is introduced.
Partition pruning also consumes block filters. It now matches named predicates to the partition column instead of assuming scan-local and stored partition positions are equal. Ambiguous dotted alias/column names conservatively skip partition pruning. The readutil debug diagnostic likewise resolves by column name rather than indexing the full schema with a scan-local position.
The leading-range optimization remains limited to supported integer, temporal, and same-scale decimal bounds. Float and byte-string leading ranges retain the original scan path because signed zero or byte-prefix ordering could otherwise cause false-negative pruning. Some first-component
BETWEENplans on those types therefore lose their former hidden-key rewrite; exact row filtering remains in place.Validation
pkg/sql/plan,pkg/sql/compile,pkg/partitionprune, andpkg/vm/engine/readutil.FilterObjectstests verify object rejection before metadata loading. Partition tests cover same-column compaction, different-column position collision, and dotted-name ambiguity.go vetandgolangci-lintpassed;git diff --checkpassed.Against base
a36da7f72d, the final diff has production net +210 lines, tests net +389, and no document changes. The planner mapping and partition identity checks account for the production increase; the tests exercise public planning and pruning behavior rather than only helper internals.The historical 329-object local reproduction establishes the missed early-pruning opportunity but did not reproduce the reported production latency. Deterministic tests establish object rejection and removal of hidden-key reader attributes; service-level latency and byte savings have not been measured. CI is pending.